Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spaces in bind-hooks paths #500

Merged
merged 2 commits into from
Jan 12, 2024
Merged

Fix spaces in bind-hooks paths #500

merged 2 commits into from
Jan 12, 2024

Conversation

Llewellynvdm
Copy link
Contributor

https://github.com/dyne/Tomb/blob/a0b633fb774626e6f6e9767b70247bf5490dd0e2/tomb#L2593-L2604

The option to target folders/files that have spaces in the name would be ideal, so I added a little work around.

@Llewellynvdm
Copy link
Contributor Author

This should also resolve #433

@Llewellynvdm
Copy link
Contributor Author

@jaromil is there a particular reason why this PR is in limbo?

@jaromil
Copy link
Member

jaromil commented Jan 12, 2024

Hi! Many thanks for your PR! It not in limbo, just some delay due to new year's holiday and the fact this project is ran by volunteers.

I am just wondering if the renaming to __ESC_SPACE__ is appropriate, or shall we just use a _ underscore for the generated targets used by bind mount.

@Llewellynvdm
Copy link
Contributor Author

The use of __ESC_SPACE__ as a placeholder in the script serves a specific purpose: it is intended to be a unique sequence that is unlikely to occur in actual file or directory names. This uniqueness is essential because the placeholder temporarily represents spaces in paths during processing to avoid issues with word splitting.

If we were to use a single underscore _, there is a real possibility that actual path names could contain underscores. These would then be erroneously converted to spaces when the placeholder is reverted, leading to incorrect path names. By using __ESC_SPACE__, we mitigate this risk because it is designed to stand out and not conflict with typical naming conventions.

Therefore, while the placeholder may seem verbose or unconventional, its distinctive pattern is a safeguard to ensure that only the intended spaces are replaced and then accurately restored, preserving the integrity of the file paths during the bind mount process.

@jaromil
Copy link
Member

jaromil commented Jan 12, 2024

Excellent explanation, thanks for all your attention to details and welcome among the Tomb contributors!

@jaromil jaromil merged commit 27792f4 into dyne:master Jan 12, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants